Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better error reporting for return, yield, return! and yield! #17792

Merged
merged 34 commits into from
Oct 7, 2024
Merged

Better error reporting for return, yield, return! and yield! #17792

merged 34 commits into from
Oct 7, 2024

Conversation

edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Sep 25, 2024 •
edited
Loading

Description

Better error reporting for return, yield, return! and yield!

Continuation of #17779

Before

Screenshot 2024-10-04 at 17 41 30

After

let f1 = return [ 3; 4 ] 
         ^^^^^^
let f2 = return! [ 3; 4 ] 
         ^^^^^^^

Before

Screenshot 2024-10-04 at 17 44 53

After

let f3 =
    [
        if true then
            "a"
            "b"
        yield! [ 3; 4 ] 
               ^^^^^^^^
    ]

Before

Screenshot 2024-10-04 at 17 46 23

After

let f4 =
    async {
        if true then
            yield "a"
            ^^^^^
        else
            yield "b"
    }

Before

Screenshot 2024-10-04 at 17 48 55

After

let maybeTask = task { return false }

let indexHandler (): Task<string> = 
    task {
        return! maybeTask
                ^^^^^^^^^
    }

Checklist

  • Test cases added
  • Release notes entry updated:

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 25, 2024 •
edited
Loading

❗ Release notes required

✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
vsintegration/src docs/release-notes/.VisualStudio/17.12.md

@edgarfgp edgarfgp changed the title Better error reporting for yield and yield! Better error reporting for return, yield, return! and yield! Sep 25, 2024
@edgarfgp edgarfgp force-pushed the better-error-reporting-yield branch from baf77ef to 33bcbe3 Compare September 26, 2024 08:25
@edgarfgp
Copy link
Contributor Author

The failing tests in this PR are the FSharp.Editor.Tests.CodeFixes.RemoveReturnOrYieldTests which are VS specific. Unfortunately I no longer own a Windows machine. @psfinaki would you able to help me with these failing test ?

@psfinaki
Copy link
Member

Hmm looking at the tests, this change indeed breaks the code fix. Although the code fix itself is quite simple, shouldn't be a big deal, hope I'll get to it later today.

@psfinaki
Copy link
Member

There you go, there is some change in range/span arithmetic there, added a few extra tests just to be sure :)

@edgarfgp
Copy link
Contributor Author

There you go, there is some change in range/span arithmetic there, added a few extra tests just to be sure :)

Thank you Sir. Enjoy the holidays :)

@edgarfgp edgarfgp closed this Sep 27, 2024
@edgarfgp edgarfgp reopened this Sep 27, 2024
@edgarfgp edgarfgp closed this Sep 27, 2024
@edgarfgp edgarfgp reopened this Sep 27, 2024
@edgarfgp edgarfgp force-pushed the better-error-reporting-yield branch from 9bbb7d6 to 49da131 Compare September 28, 2024 13:43
@edgarfgp edgarfgp force-pushed the better-error-reporting-yield branch from 49da131 to f98e2dd Compare September 28, 2024 15:41
@edgarfgp edgarfgp closed this Sep 30, 2024
@edgarfgp edgarfgp reopened this Sep 30, 2024
@edgarfgp edgarfgp force-pushed the better-error-reporting-yield branch from c7b3589 to 40a94ad Compare October 3, 2024 18:29
@edgarfgp edgarfgp force-pushed the better-error-reporting-yield branch from 2ca244d to 5d3572e Compare October 5, 2024 22:00
@edgarfgp
Copy link
Contributor Author

edgarfgp commented Oct 6, 2024

This is ready

@edgarfgp edgarfgp marked this pull request as ready for review October 6, 2024 10:08
@edgarfgp edgarfgp requested a review from a team as a code owner October 6, 2024 10:08
@vzarytovskii vzarytovskii merged commit e017a71 into dotnet:main Oct 7, 2024
32 checks passed
This was referenced Oct 31, 2024
Closed
Closed
@dotnet-bot dotnet-bot mentioned this pull request Nov 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@T-Gro T-Gro T-Gro approved these changes

@vzarytovskii vzarytovskii vzarytovskii approved these changes

Assignees
No one assigned
Labels
None yet
Projects
F# Compiler and Tooling
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@edgarfgp @psfinaki @vzarytovskii @T-Gro